-
Notifications
You must be signed in to change notification settings - Fork 8.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
YARN-7237: Enable asynchronous scheduling by default for capacity scheduler #7138
base: trunk
Are you sure you want to change the base?
Conversation
Submitted a first cut PR to trigger unit tests for Yarn capacity scheduler with async scheduling. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@slfan1989 @aajisaka @TaoYang526 Could you please review the changes ? Note: #7129 is a blocker for this change to now in. |
@shameersss1 Thanks for your contribution. |
@slfan1989 - Gentle reminder for the review |
// disable async-scheduling for simulating complex scene | ||
Configuration disableAsyncConf = new Configuration(conf); | ||
disableAsyncConf.setBoolean( | ||
CapacitySchedulerConfiguration.SCHEDULE_ASYNCHRONOUSLY_ENABLE, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why disable async scheduler in UT? And enable by default? I think we'd better enable async schedule by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following are the reasons why async scheduling is disabled in UT :
-
unlike async scheduling, sync scheduling gives more flexibility while writing UTs to mimic complex scenarios since the UT writer will have full control over when to schedule, where as with async scheduling this cannot be done.
-
There were 200+ failing when async scheduling was enabled (due to scheduling assertion)
Description of PR
Enable asynchronous scheduling by default for capacity scheduler.
This commit enables asynchronous scheduling by changing the config default value to true. Additionally the commit also adds documentation changes for different scheduling strategies.
Note: Although asynchronous scheduling is enabled by default, for unit testing disabling it by default to give more control over container scheduling while simulating complex tests (i.e synchronous scheduling gives more control over when to schedule the container for simulating certain cases while writing unit test).
How was this patch tested?
Existing Unit Tests
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?